Make the outstanding-issues snapshot merge-safe (v2), and record the measured assessment of two other rows - #2735
Conversation
… revision, drop the sha Closes the second half of #Y090R5. The `pending` half was fixed already — the committed artefact carries an empty `pending` and only `prebuild` asks for the live list. `ledger_revision` is what was left, and it is the only field in this file measured changing as a side effect of unrelated work: PR #2690 ("calculators: remove directive clinical copy"), which touches no ledger file, rewrote exactly those two lines and nothing else here. The churn is structural, not occasional. A reconciliation regenerates the snapshot and then becomes the ledger's newest commit, so the value it just wrote is stale the instant it lands — which is why the gate excludes `ledger_revision` from comparison in the first place. Every later branch that regenerates (pre-commit doc sync, docs:update, next build) rewrites the field with whichever ledger commit ITS base carries. Two branches cut either side of a reconciliation write different values into adjacent lines at the top of the file. Excluding a field from the gate never stopped it conflicting in git, because the bytes still shipped. Proven by simulating that exact sequence — a ledger commit, one branch cut before it and one after, each regenerating during its own unrelated commit: BEFORE (v1: sha + timestamp): CONFLICT (content) in data/outstanding-issues-snapshot.json AFTER (v2: date only): CLEAN MERGE The sha goes because nothing reads it: `resolveFreshness` uses `committed_at` alone. The timestamp coarsens to a day, so two branches regenerating on the same day write identical bytes — and same-day is the measured case, the two commits behind #2690's rewrite being 35 minutes apart. Branches a day apart still differ, which is deliberate residue: freshness is the one value here a reader cannot recompute. `counts` is deliberately NOT removed, unlike its repo-awareness sibling. It is derived from the canonical ledger alone, and ledger edits are serial by policy (one reconciliation branch), so it is not a contended surface — 9 commits in 60 days, all but one of them reconciliations. Removing it would be a larger change than the evidence asks for. Same device and reasoning as `captured_revision` in repo-awareness-snapshot-v3, which closed the identical defect in the sibling file. Version bumped to outstanding-issues-snapshot-v2 so a stale v1 snapshot fails `loadLedgerSnapshot` loudly rather than rendering a shape the reader no longer expects. `readCommittedRevision` normalises a v1 revision it finds on disk, because the preserve-instead-of-read path is taken by the production image (no `.git`) and would otherwise write the conflicting v1 shape straight back into a v2 file — pinned by a new test. No test deleted, skipped or quarantined. Four fixtures moved to the v2 shape; one test added for the normalisation above. Verification: - npm run test: 1269 files, 18067 passed, exit 0 - npx tsc -p tsconfig.json --noEmit: exit 0 - eslint clean on every changed path; npm run format committed - node scripts/check-outstanding-issues-snapshot.mjs: in step (121 open, 0 pending) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmcnnSgi8sxy7dQipsYCWG
…c rows Two rows filed on 2026-09-06 were re-assessed against origin/main 0177bed rather than fixed, because measurement showed neither needs the fix its row implies. #KHTTW4 (Ward Flow): the eight failures are gone — 71 passed, 3 skipped, 0 failed. The path-scoped blind spot the row exists for is designed out, the lane now gating on ui_changed rather than ward paths, but it is held inert behind vars.WARD_JOURNEYS_BLOCKING until somebody has a green run in hand. That precondition is now met and the run is recorded, so the one remaining action is a repository-settings toggle no PR can perform. #RA0QAH (flaky Production UI specs): the standing worry its own text raised — that the Next 16 prefetch-header trap might sit elsewhere — is closed by a repository-wide grep returning only the already-fixed site. The other three failures stay unexplained, and deliberately unfixed: the row never named the specs, the CI runs are past useful retention, and a fix without a reproduction is a guess that risks quarantining something real. Queued as merge-safe inbox requests; reconcile after this lands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmcnnSgi8sxy7dQipsYCWG
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_792c07c6-15a3-4628-9d37-2e4f66e1a9e3) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 274f2ac96e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ock time and a false hour count
Codex review finding on this PR (P2), confirmed by reading the render path
rather than taken on trust — and it is worse than reported: the same defect
is already live on main for the repo-awareness snapshot, whose
`captured_revision` has been a date since v3. One fix in FreshnessStamp
covers both, because line 82 was the only snapshot-mode render of ageHours.
Two false claims, both from parsing a date as midnight UTC:
- `new Date("2026-09-07")` formatted with `timeStyle` in Australia/Perth
renders "7 Sept 2026, 08:00 AWST". Nothing recorded 08:00. This component
exists to stop confident-looking stamps carrying no information, so
manufacturing one is squarely the failure it guards against.
- `ageHours` measured from that midnight is wrong by however far into the
day the commit actually was, up to a full day, so a ledger update can read
as "24 hours old" minutes after landing.
A date-only value now renders as a date, with no clock time and no zone,
and ages in whole days. A full timestamp keeps hour precision, pinned by a
test so this cannot quietly coarsen the live ingestion stamp.
The day arithmetic is UTC on both sides, and correcting that was a genuine
bug in my own first attempt, caught by the 23:59 test rather than by
reading. The stored date comes from a commit's `%cI`, so it is a UTC
calendar date; comparing it against the reader's Perth date mixes zones and
adds a further day of error on top of the day already lost to rounding —
23:59 UTC is 07:59 next morning in Perth, which called a just-landed commit
"1 day old". Rendering stays pinned to Perth like the rest of the
component, and midnight UTC falls on the same calendar date there anyway.
Four tests added: no fabricated clock time, the reported 23:59 case reading
"same day", singular "1 day old", and hour precision retained for a real
timestamp. No test deleted, skipped or quarantined.
Verification:
- npm run test: 1269 files, 18071 passed, exit 0
- npx tsc -p tsconfig.json --noEmit: exit 0
- eslint clean on both changed paths; npm run format committed
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KmcnnSgi8sxy7dQipsYCWG
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ae4dd56d-a6e5-4825-9451-1c87599e4d25) |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #16665 (failure). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_d2b65a5e-b562-4b11-873e-42eb007a0981) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_49d5192f-5eb5-4147-ba9e-cd2cd979f179) |
PR Unblock summary (#2735)Head: Fixes this passNone required. Diff inspected (snapshot v2 date-only Thread status
Verification
Required CI (exact head)All green. Pending required ≠ applicable. Advisory / baseline / infra (separate)
Residual risk
Merge-ready: yes (required exact-head checks green, mergeable, no actionable threads). Final merge decision left to the user. The PR was not merged. Final merge is left to the user. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_7569b0ef-89ac-4fdf-becb-efeb4331330c) |
PR Helper adversarial reviewActions: Confirmed mergeable; Review evidence
Adversarial verdict — GO_WITH_CAVEATSCaveat is land-order vs #2770, not a safety defect in the v2 design. Chief ping after required CI green and sequencing decided. The PR was not merged. Final merge is left to the user. |
Adversarial review (outstanding-issues snapshot v2)Head: Evidence
Verdict: GO_WITH_CAVEATSCaveat is land-order vs #2770: this PR writes v2 date-only revision; #2770 still writes v1 The PR was not merged. Final merge is left to the user. |
Resolve outstanding-issues-snapshot.json for post-#2770 main while keeping v2 date-only ledger_revision (no sha). Regenerated snapshot matches merged ledger counts.
|
Unblocked after #2770 landed on main.
#2770 was already MERGED (08:34:25Z); no further push on that PR. The PR was not merged. Final merge is left to the user. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2aaaa544-3d91-44d8-8917-de770fa776c7) |
BigSimmo
left a comment
BigSimmo
left a comment
There was a problem hiding this comment.
Copilot-style review — exact head ebe1a8a51697…
Focus: ledger/snapshot integrity / silent shape reintroduction / cross-PR consistency.
Verdict: COMMENT
v2 day-only ledger_revision (no sha) is the right merge-safety fix; mirror of repo-awareness-snapshot-v3. Gate still ignores ledger_revision (necessary). Production preserve path normalises v1→date and is pinned by test — closes the silent sha reintroduction hole. FreshnessStamp day-vs-hour honesty is correct.
Findings
-
P2 — Stale
#KHTTW4inbox instruction vs concurrent #2710. Inbox update still says owner must setvars.WARD_JOURNEYS_BLOCKING=true. #2710 one90815ccremoves that variable and enables the lane in workflow. Landing this after #2710 leaves a misleading “next action”; landing before leaves contradictory CI story. Refresh the inbox payload to match whichever enablement path wins (settings var vs code enable), or mark superseded by #2710. -
P3 — Cross-day conflicts remain by design. Day coarsening fixes same-day churn (#2690 case). Branches regenerating on different UTC days still conflict on
committed_at. Acceptable residue if documented; do not claim “merge-safe forever.” -
P3 — Snapshot
committed_at: 2026-09-10on a 2026-09-12 head. Expected if ledger untouched; freshness UI will age in days. Ensure reconcile/regenerate happens when inbox rows land so counts/open stay honest (PR notes reconcile-after-land — good). -
P3 (positive): Keeping
counts(unlike awareness sibling) is justified by serial ledger edits; v1 load fails loud via version constant — good fail-closed.
Never merge from this review.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b62cf8f4-070b-43ad-b680-cf161a69c581) |
Drop the instruction to set WARD_JOURNEYS_BLOCKING; #2710 enables ui-ward-journeys in ci.yml and removes that repo-var gate.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_293d72c0-a5d2-4bb9-8689-47833e8d9aa0) |
2 participants
Summary
Three ledger rows were re-assessed against
origin/main0177bed before any code was written. One needed a fix, and got one. Two did not, and are recorded rather than "fixed" into something they no longer are.#Y090R5sibling snapshotrepo-awareness-snapshot-v3#KHTTW4Ward Flow#RA0QAHflaky specs1. The one real defect:
data/outstanding-issues-snapshot.jsonThe
pendinghalf of#Y090R5was already fixed — the committed artefact carries an emptypending, and onlyprebuildasks for the live list.ledger_revisionis what was left, and it is the only field here measured changing as a side effect of unrelated work: PR #2690 ("calculators: remove directive clinical copy") touches no ledger file and rewrote exactly those two lines and nothing else in this snapshot.The churn is structural rather than occasional. A reconciliation regenerates the snapshot and then becomes the ledger's newest commit, so the value it just wrote is stale the instant it lands — which is why the gate excludes
ledger_revisionfrom comparison in the first place. Every later branch that regenerates (pre-commit doc sync,docs:update,next build) rewrites the field with whichever ledger commit its base carries. Excluding a field from the gate never stopped it conflicting in git, because the bytes still shipped.Proven by simulating that exact sequence — a ledger commit, one branch cut before it and one after, each regenerating during its own unrelated commit:
The sha goes because nothing reads it:
resolveFreshnessusescommitted_atalone. The timestamp coarsens to a day, so two branches regenerating on the same day write identical bytes — and same-day is the measured case, the two commits behind #2690's rewrite being 35 minutes apart. Branches a day apart still differ, which is deliberate residue: freshness is the one value here a reader cannot recompute.countsis deliberately NOT removed, unlike its repo-awareness sibling. It derives from the canonical ledger alone, and ledger edits are serial by policy, so it is not a contended surface — 9 commits in 60 days, all but one of them reconciliations. Removing it would be a larger change than the evidence asks for.readCommittedRevisionnormalises a v1 revision found on disk, because the preserve-instead-of-read path is taken by the production image (no.git) and would otherwise write the conflicting v1 shape straight back into a v2 file. Pinned by a new test.2. Ward Flow (
#KHTTW4) — do not fix, it is already fixednpm run test:e2e:ward-journeyson a freshorigin/mainworktree: 71 passed, 3 skipped, 0 failed. The eight failures are gone.Two of the three skips are the
ui-ward-morningpair, and that skip is an owner-approved decision rather than a silencing: MERGE 02 folded the morning board intoCapacityScreen,MorningPageis unmounted, andmorning-page.tsx's own comment forbids retargeting the spec or re-mounting the component pending the owner's ruling on spec D9. Component coverage continues intests/ward-morning-page.dom.test.tsx(20 cases) andtests/ward-morning-print.test.ts.The durable half — a path-scoped gate cannot report a break in code already broken on its base — is designed out:
ui-ward-journeysnow gates onneeds.changes.outputs.ui_changed, not ward paths. It is held inert behindvars.WARD_JOURNEYS_BLOCKING, deliberately, so that turning it on is one act taken by somebody holding a green run. That precondition is now met, and the green run is recorded on the row. Setting that variable is a repository-settings change no pull request can make, so it is left to the owner.3. Flaky Production UI specs (
#RA0QAH) — one half closed, one half honestly leftThat row's own text left a standing worry: the Next 16 prefetch-header trap is generic to any Playwright assertion trying to tell a prefetch from a navigation, and was worth grepping for. Grepped: two hits, both inside the already-fixed block. No second occurrence exists, so no further code fix follows.
The other three failures stay unfixed on purpose. The row never named the specs, the only pointers are CI runs past useful retention, and a fix without a reproduction is a guess — which is how a real failure gets quarantined by accident. Recorded as a watch item with the exact input to capture next time.
Verification
npm run test— 1269 files, 18067 passed, 2 expected fail, 3 skipped, exit 0npx tsc -p tsconfig.json --noEmitexit 0npm run formatrun whole-tree and committednode scripts/check-outstanding-issues-snapshot.mjs—in step with data/outstanding-issues-snapshot.json (121 open, 0 pending)npm run test:e2e:ward-journeys— 71 passed, 3 skipped, 0 failed (the evidence behind §2)npm run verify:pr-local— Verification not run: deferred to CI. The full offline suite above is the widest gate this change can break.No test was deleted, skipped or quarantined. Four fixtures moved to the v2 shape; one test added for the v1 normalisation path.
Risk and rollout
DeveloperAreaGate; no clinical, retrieval or patient-facing surface is touched. The version bump means a stale v1 snapshot failsloadLedgerSnapshotloudly rather than rendering an unexpected shape.Notes
docs/superpowers/still name v1. They are a record of what was built then and are deliberately left alone.🤖 Generated with Claude Code
https://claude.ai/code/session_01KmcnnSgi8sxy7dQipsYCWG
Generated by Claude Code
Note
Low Risk
Changes are confined to developer-hub snapshot data, its generator, and freshness UI; a stale v1 snapshot fails loudly at load with no patient-facing impact.
Overview
Outstanding-issues snapshot v2 stops unrelated branches from merge-conflicting on
data/outstanding-issues-snapshot.json:ledger_revisiondrops the unused git SHA and stores only a UTC calendar date (matching the repo-awareness snapshot approach). The generator normalizes legacy v1 revisions on the no-git production preserve path, and types/tests/gate fixtures move tooutstanding-issues-snapshot-v2.FreshnessStamp now treats day-only revision strings honestly—date-only display without a fabricated AWST clock time, and age shown in whole UTC days instead of misleading hour counts from midnight parsing. Full timestamps still use hour precision.
Two merge-safe inbox updates record reassessments for ledger rows
#RA0QAH(prefetch grep closed; three flaky specs remain watch-only) and#KHTTW4(ward journeys green; CI enablement tracked pending #2710)—not canonical ledger edits in this diff.Reviewed by Cursor Bugbot for commit e90d6ee. Configure here.
Current consolidation status (2026-09-10)
The current head includes main and passed 60 focused snapshot/developer tests. Hosted run 34495136858 passed static, build, safety and production UI checks; unit coverage passed 18,155 tests and failed only
tests/ranking-snapshot-provenance.test.tsbecause the base ranking fixture is 32 days old.Merge prerequisite: #2763 carries the existing ranking fixture refresh. Keep this PR open until that prerequisite and the required aggregate are green; do not weaken the 30-day provenance requirement or trigger a redundant rerun on this unchanged head.